-
Notifications
You must be signed in to change notification settings - Fork 0
Add basic functionality for user service #9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Dynamic API client creation to handle different endpoints for server-side (container-to-container) vs client-side (API gateway) requests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Log in, log out and signup work as expected, have some super minor comments on validation checks beforehand to handle some exceptions if any ever arise, but u can choose to ignore bc they r vv minor
Running npm audit
within user-service shows that there are several dependency vulnerabilities:
8 vulnerabilities (4 low, 3 high, 1 critical)
maybe these can be fixed b4 we develop Peerprep further, running npm audit fix
should fix it and abit of testing can be done to make sure the updates didn't break anything
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Tested sign up, login, logout, creating new tabs and everything works as intended.
There is a minor issue with duplicate username creation during signup described below.
Regarding the routing, should we only allow authenticated users to access the landing page? ( Currently unauthenticated users can visit the landing page without logging in)
…l/username on sign up
Review Comments:
Bugs encountered:
Future improvements to consider: |
# Conflicts: # frontend/src/app/match/page.tsx # frontend/src/app/page.tsx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will be maintaining use of single domain
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Functionality Added
Others Changes
Known Issues:
The local mongodb yet to be tested. (maybe we can just remove it if we all ok with it)
Yet to do:
Sending verification link to email
Other notes:
Get the .env file in user service from me got the monbodb secret.